Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

@coderabbitai, full review.

#!/bin/bash
set -euo pipefail

# Get PR details and diff
gh pr view 22 --repo cppalliance/boost-docs-translation --json title,body,files,additions,deletions

echo "===== DIFF ====="
gh pr diff 22 --repo cppalliance/boost-docs-translation

#!/bin/bash
set -euo pipefail

# Get the PR branch files
gh api repos/cppalliance/boost-docs-translation/contents/.github/workflows/assets/lib.sh \
  --jq '.content' | base64 -d

echo "===== add-submodules.yml ====="
gh api repos/cppalliance/boost-docs-translation/contents/.github/workflows/add-submodules.yml \
  -H "Accept: application/vnd.github.raw" 2>/dev/null || \
gh api repos/cppalliance/boost-docs-translation/git/trees/HEAD --jq '.tree[] | select(.path | contains("add-submodules"))' 2>/dev/null || true

echo "===== start-translation.yml ====="
gh api repos/cppalliance/boost-docs-translation/contents/.github/workflows/start-translation.yml \
  -H "Accept: application/vnd.github.raw" 2>/dev/null || true

@AuraMindNest, here is the full review of PR #22.

Review: Extract secret validation to lib.sh

Overall: The centralization is well-structured. The three new helpers (require_lang_codes, _require_nonempty, validate_secrets) follow a clean hierarchy. Below are the issues I found, ordered by severity.

🔴 Medium — Fail-fast regression in start-translation.yml

In the old code, GITHUB_TOKEN, LANG_CODES, WEBLATE_URL, and WEBLATE_TOKEN were all validated at the top of the "Start translation" step before any sourcing. After this PR, validate_secrets weblate is called after sourcing three library files:

source "$GITHUB_WORKSPACE/.github/workflows/assets/env.sh"
source "$GITHUB_WORKSPACE/.github/workflows/assets/lib.sh"
source "$GITHUB_WORKSPACE/.github/workflows/assets/translation.sh"
validate_secrets weblate    # ← validation deferred to here

If translation.sh ever reads WEBLATE_URL or WEBLATE_TOKEN at source time (e.g., for initialisation), it would fail with a confusing error before the clear validation message fires. Consider moving validate_secrets weblate immediately before sourcing translation.sh (but after env.sh/lib.sh, since those provide ORG, MODULE_ORG, etc. that validate_secrets checks):

source "$GITHUB_WORKSPACE/.github/workflows/assets/env.sh"
source "$GITHUB_WORKSPACE/.github/workflows/assets/lib.sh"
validate_secrets weblate    # ← move here, before translation.sh
source "$GITHUB_WORKSPACE/.github/workflows/assets/translation.sh"

This matches the intent of the header comment: "Call validate_secrets … after sourcing env.sh and lib.sh."

🟡 Minor — Unrelated rename of trigger_weblate parameters

The diff includes a rename of two local parameters inside trigger_weblate():

-  local weblate_url="$1" weblate_token="$2" libs_ref="$3" exts_json="$4"
+  local api_base_url="$1" api_token="$2" libs_ref="$3" exts_json="$4"

This is functionally correct (positional call-site is unchanged) but is unrelated to the PR's stated goal of extracting validation. It adds noise to the diff and may confuse reviewers. Consider splitting it into a separate commit or noting it in the PR description.

🟡 Minor — _require_nonempty: SC2154 disable scope

_require_nonempty() {
  local var_name="$1" err_msg="$2"
  # shellcheck disable=SC2154
  [[ -n "${!var_name:-}" ]] || { echo "$err_msg" >&2; exit 1; }
}

The placement of # shellcheck disable=SC2154 is correct (it suppresses the next line). However, SC2154 is about a specific undeclared variable — shellcheck may not actually fire it here because var_name is explicitly declared as local. The real suppression needed might be SC2034 (variable appears unused) or none at all. Worth verifying with shellcheck -x locally to confirm the exact code being suppressed.

🟢 Nit — ${!var_name:-} — redundant :-

[[ -n "${!var_name:-}" ]]

Since var_name is a declared local parameter (always bound), ${!var_name:-} is equivalent to ${!var_name} here. The :- default is harmless and adds set -u safety, but could be documented with a brief comment to explain intent.

✅ What looks good

• require_lang_codes cleanly extracts the duplicated pattern with the exact same error message as before, so existing tooling/log parsers are unaffected.
• validate_secrets [weblate] flag design is clean — single optional argument gates the Weblate block without duplicating the base checks.
• Header comments in lib.sh now document the expected sourcing order, which is valuable for future maintainers.
• add-submodules.yml ordering is correct: validate_secrets is called after env.sh+lib.sh are sourced, ensuring all variables are available for validation.
• The PR fully closes issue #20 — all inline validation blocks in both workflow files are removed.

Add a few bats cases in tests/test_lib.bats for .github/workflows/assets/lib.sh — require_lang_codes & validate_secrets using load_env and unset/export patterns.